Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMPT-46] Update Crashlytics #34

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

ivanjiang5628
Copy link

@ivanjiang5628 ivanjiang5628 commented Nov 23, 2020

Description

Because The Fabric SDK is now deprecated and will not continue reporting the app's crashes.
I removed the Fabric SDK and added the latest Firebase Crashlytics SDK to the app by following the guides.

image

General PR Class

🐛 = Bug Fix (Fixes an Issue)

Release Note

Dependencies / ENV

Risk Scorecard

  1. As the author you should check the boxes that correspond with your PR and then use the following guide to set your risk label:
  • 0 checkboxes => low risk
  • 1-3 checkboxes => medium risk
  • 4+ checkboxes => high risk
  1. Unless exempt, checked risk factors should be explained comprehensively in the Release Risk Assessment section below
  2. Medium or higher risk PRs should get more than one code-review approval

NOTE: if you aren't changing any production files, please use the zero risk label

  • requires env configuration to be added in production
  • js package changes1
  • more than 200 LOC changed in production files1
  • includes a user-facing workflow change to an existing production feature (user muscle memory or pattern recognition will be affected)
  • could prevent access to Jane Video (eg. cors, middleware, changes to auth system)
  • affects a widely used component or piece of code
  • I have a doubt - I want the RMT to review this. If possible, please elaborate your concerns in the risk assessment section.

1 No need to explain these risk factors below

Release Risk Assessment

Low

Code Review

Resource: Dev Team Notion Page
Resource: Code Review Checklist

  • I clearly explained the WHY behind the work, in the Description above

Design

  • I added instructions for how to test, in the QA section below
  • I added specs for changes, or determined that none were required
  • I demoed this to the appropriate person
  • I considered both mobile & desktop views, or that wasn't relevant

Code

  • I committed code with informative git messages
  • I wrote readable code, or added comments if it was complex
  • I performed a self-review of my own code
  • I rebased my branch on the latest master

@ivanjiang5628 ivanjiang5628 self-assigned this Nov 23, 2020
@ivanjiang5628 ivanjiang5628 force-pushed the TEL-293-Update-Crashlytics branch from 11dfbf6 to 4239d35 Compare November 24, 2020 17:41
@ivanjiang5628 ivanjiang5628 requested a review from gerbus November 24, 2020 17:50
@ivanjiang5628 ivanjiang5628 added the Risk - Low Low risk of disruption to codebase or infrastructure. label Nov 24, 2020
Copy link

@gerbus gerbus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tough to code review these files, but looks about right to me. I don't see a QA section in your PR write-up, but I imagine you did some developer QA? In /janeapp/jane we're starting to move towards documenting the developer QA in that section. I'm happy if you just confirm that you tested this, but for the future something to consider.

@ivanjiang5628
Copy link
Author

Tough to code review these files, but looks about right to me. I don't see a QA section in your PR write-up, but I imagine you did some developer QA? In /janeapp/jane we're starting to move towards documenting the developer QA in that section. I'm happy if you just confirm that you tested this, but for the future something to consider.

👍 Hi, Chris Yes I did some developer QA I think it would be great to have the developer QA section in this repo!

@gerbus
Copy link

gerbus commented Dec 3, 2020

Tough to code review these files, but looks about right to me. I don't see a QA section in your PR write-up, but I imagine you did some developer QA? In /janeapp/jane we're starting to move towards documenting the developer QA in that section. I'm happy if you just confirm that you tested this, but for the future something to consider.

👍 Hi, Chris Yes I did some developer QA I think it would be great to have the developer QA section in this repo!

Great! Once the PR template is updated in janeapp/jane I'll try to remember to port the relevant stuff over here too 👍

@clem-jane clem-jane changed the title [TEL-293] Update Crashlytics [IMPT-46] Update Crashlytics Feb 11, 2021
@linear
Copy link

linear bot commented Feb 11, 2021

IMPT-46 Update Crashlytics

Firebase will be deprecating older versions of Crashlytics SDK and moving away from the old Fabric ones as well. Based on the Firebase docs, we should update to more recent SDKs anyway
https://firebase.google.com/docs/crashlytics/upgrade-sdk#add-crashlytics-sdk

View original issue in Jira

@clem-jane
Copy link

@jngo-janeapp could we get your help to QA this?

@jngo-janeapp jngo-janeapp self-assigned this Feb 11, 2021
@jngo-janeapp
Copy link

@jngo-janeapp could we get your help to QA this?

Hey @clem-jane don't think there is much for me to QA here as its been developer QA'd as per the notes above. I'll speak with Ivan when he's back to see if there is anything I can test on my end @ivanjiang5628 thanks!

@ivanjiang5628 ivanjiang5628 force-pushed the TEL-293-Update-Crashlytics branch from 4239d35 to f1fd281 Compare February 24, 2021 18:57
@ivanjiang5628 ivanjiang5628 merged commit 6a657a3 into react-native-prod Feb 24, 2021
ivanjiang5628 pushed a commit that referenced this pull request Nov 4, 2021
* feat(polls) Added boilerplate code for polls feature

* feat(polls) Implemented simple poll creation and answer modals in web app

feat(polls) Added button to create a poll in toolbar
feat(polls) Added Modal to answer an incoming poll
feat(polls) Implemented basic client-side sending and reception of polls
feat(polls): linked Poll creation to poll answering
fix(polls) Linted code
feat(polls.create) Added fields for question and answers (#3)
* feat(polls.create) Added fields for question and answers + keyboard navigation
* feat(polls.create) Minor changes, added some comments
feat(PollAnswer Component): Component to display modal to answer poll #1 (#2)
* fix(polls) removing necessity of current_poll_id variable
* fix(polls) linting, polls are now updated when an answer is sent
* feat(polls answer) added translation
* fix(polls answer) remove extra comments, fixed typo
* improvement (polls answer) use useSelector instead of mapStateToProps. cleaner code
* fix (polls create) renamed sender to senderId
* fix (polls answer) turned arrow function into useCallBack
feat(PollResults Component): Component to display poll results (#1)
* feat(PollResults Component): fist version of the component
* feat(detailed votes): Display the detailed results of a poll
* feat(Poll results): Use display name instead of ids in detailed results mode
* fix(Poll): change title to question
* fix(Poll type): import Poll type from types.js
* fix(Poll): change title to question
* fix(Poll): get participants out of the map
* fix(Poll): replace filter with find
feat(polls.create) Added "+" and "x" buttons in poll creation form + improved keyboard navigation a bit
feat (polls) Answer modal now display results in real time after validation or skip
feat(polls.create) Minor improvements to poll creation form
feat(poll result) Added default message when trying to display no answer
fix (polls) result windows is now small by default
fix (polls) sanitizes imports to allow startup on react native

* feat(polls.native) Implemented native toolbar button & poll create modal

feat( poll native) added poll creation button in native toolbar
improvement(polls) only one file used for PollCreateButton
feat (polls native) added an example dialog
feat (polls native) added possibility to create and delete options in poll creation
improvement (polls) better styling for PollCreateDialog

* feat(polls) Added ability to drag&drop answers in web poll creation form

* feat(polls) Added native poll answer modal + chat integration, refactored components

Merge branch 'polls-native' of https://github.com/jade-guiton/jitsi-meet into polls-native
improvement (poll) Better styling for poll answer, now uses icons
feat(poll.PollResults): Add native version of PollResults
feat(poll.PollResults): Post results in chat in Native
fix(poll.PollResults): Fix linter error in ChatMessage
feat(polls.native) Improved styling for native poll answer dialog (required some internal changes)

* fix(polls) Heavily refactored and added bars to poll results, other minor changes

fix(poll.create): Move title to Dialog title
feat(poll.create) Minor changes to poll creation / answer dialogs
fix(poll.create) Refactored and improved translations
feat(poll) Improved CSS for modals in web version
fix(poll.pollcreate): Fix button size in native
fix(polls) Refactored poll results component and other minor changes
fix (polls) remove double import
refactor(poll) Heavily refactored poll results (native + web)
feat(polls.results) Added percentage bars and vote counts in web poll results, minor changes to mobile poll results

* fix(polls) Fixes and linting

fix(polls) Reformatted and fixed some linter and Flow errors
fix(polls.results) Fixed voter list border appearing with 0 voters

* feat(polls): Add modal with detailed votes that can be open from the result summary in the chat

* fix(polls) Fixes, refactorings, and minor design changes

feat(polls.results): Refactored poll chat message and improved design in web app
feat(polls.results) Same as last commit, but for mobile version
refactor(polls.results) Refactored PollResultsMessage and removed unnecessary prop in PollResults
fix(polls.results) Fixed all remaining linter and Flow errors
improvement(polls) removed console logs, added comments
fix (polls) linting
fix(polls.results) Fixed bug with poll chat message displaying the wrong name
feat(polls.results) Minor improvement on poll results display (web)
fix(poll.results): Use getParticipantDisplayName to get participant name and avoid empty string as name

* Feat(poll.results): Remember voters names to display after they left the conference (#10)

* feat(poll.results): Add the sender name in Poll object to remember names if participants leave the conference. Names are also updated if changed
* refactor(poll.results): Refactor the memorization of the names of voters to use the same logic as in  the chat
* refactor(poll.results): use Map instead of Array.From(
* refactor(poll.answer): change the way names are stored in poll answers to persist if participant left the call
* Update react/features/polls/components/AbstractPollAnswerDialog.js
* Update react/features/polls/components/AbstractPollCreateDialog.js
* refactor(poll.answer): use voterName instead of senderName to avoid confusion with senderId the id of the sender of the poll
* improvement(polls) Simplified poll answer voter name logic

Co-authored-by: Fabien Zucchet <[email protected]>
Co-authored-by: Jade Guiton <[email protected]>

* fix(poll.native): Fix UI overflow when asking long questions & long options in the mobile app (#11)

Co-authored-by: Fabien Zucchet <[email protected]>

* fix(polls) Fixed close button behavior in answer and results dialog (#12)

* fix(polls) Fixed close button behavior in answer and results dialog
* fix(polls) Fixed linter error

* fix(polls) Added a poll queue to avoid overwriting open modals (#13)

* fix(polls) Added a poll queue to avoid overwriting open modals
* fix(polls) Updated documentation for action RECEIVE_POLL

* Refactor(poll.chatresults): Add message in chat with hidden results until the participant has answered (#14)

* refactor(poll.chat): Display poll results in chat when the poll is created instead of when the participant has ansered
* refactor(poll.chat): Hide results until the participant has answered, skipped or canceled a responde to the poll
* Use getParticipantDisplayName instead of only getStore()
* Hide results also in native
* fix(polls) Fixed previous merge

Co-authored-by: Fabien Zucchet <[email protected]>
Co-authored-by: Jade Guiton <[email protected]>

* minor improvements (polls)

refactor (polls) uniformized string for command names
refactor (polls) changed pollId type to number everywhere

* feat(polls) Added persistence to polls using sendMessage instead of sendCommandOnce (#16)

* feat(polls) Using sendMessage instead of sendCommandOnce, switched poll IDs to string, and ability to receive old polls from backend
* improvement(polls) Linted everything, fixed Flow errors, and added Prosody plugin for polls
* improvement(polls) Historic polls are now displayed in chronological order

* (polls) Minor improvements (#17)

* renaming (polls) Renaming senderId -> voterID for voters
* improvement (polls) sender's name is now provided with poll
* comments (polls) updated comments for senderName types
* fix(polls) Finished merging with json-messages feature
* fix(polls) Fixed incorrect json-message sent with 0 polls

Co-authored-by: Jade Guiton <[email protected]>

* Move polls to tab (#23)

* Draft(polls): Move polls to polls-pane ; first version for web
* Draft(polls): Move polls to polls-pane ; clean styled.js and remove Participant objects
* fix missing newline at the end of file
* Change behaviour to allow answer poll later
* Fix(polls): change pollId type from number to string for consistency
* feat(polls-pane): Ability to answer to a poll in polls-pane
* feat(polls-pane): Ability to create to a poll in polls-pane
* feat (polls.pane) display a notification when a new poll arrives
* refactor(polls-pane): Update CSS to have a design closer to the mockups
* fix(poll.vote count): Fix votes counting when computing percentage
* fix(poll.vote count): Fix votes counting when computing percentage
* refresh fork with jitsi/jitsi-meet
* design (polls) Better look for poll creation
* refactor(polls pane): Move polls-pane as a chat tab
* Remove the first version of the polls-pane and the button to open it
* Fix notifications and typo
* Translate new polls tab in chat
* Change polls_pane to polls-pane
* Remove unless functions
* Remove usage of styled.js
* Improve responsiveness
* Separate web and native logic
* Remove Create a Poll button in web toolbox
* improvement (polls) added auto scrolling to bottom when a new poll arrives
* Add tabs to swicth between polls and chat in native
* Add AbstractPollsPane
* Add AbstractPollCreate
* Add AbstractPollAnswer
* Add PollAnswer, PollItem and PollList for native
* Add PollCreate for native
* Remove dialogs in web and native
* Remove dialog queue
* Remove useless files
* Move _polls.scss outside dialog folder
* Add possibility to skip answer
* Add (useless for now) see details link
* Add possibility to show detailed results for a poll
* Resize progress bar to make details display
* refactor, design (polls) better style to native design chat
* fix (polls) Removed unecessary files
* translate (polls) added french translation to empty polls
* design fix (polls.native) 'show details' now correctly switch between progress bar and voters mode
* Change See detailed results for Show details and add cursor: pointer
* Fix progress bars not aligned with text
* fix (polls.native) added autoselection of newly created option
* Remove poll answer
* improvement(polls.create) Improved web poll creation form marginally
* improvement(polls.change) Simplified answer removal by reusing poll-answer command
* fix linter
* Fix(translation): update translation

Co-authored-by: Fabien Zucchet <[email protected]>
Co-authored-by: spineki <[email protected]>
Co-authored-by: Fabien Zucchet <[email protected]>

* Merge pull request #22 from jade-guiton/polls-with-notification

feat (polls) chat notification badge now display the sum of unread  messages and unread polls
fix(translation): Fix missing translation
Fix flow error

* Cleaned up, fixed, and uniformized translations

* Small improvements to PollAnswer and PollResult + Much refactoring

Specifically:
- "Change vote" button now says "Vote" if voting was skipped
- Clicking on "Change vote" resets the voting form to the last submitted answers instead of a blank slate

- The "answered" field of Polls was replaced by "showResults" and "lastVote"
- The "setAnsweredStatus" action was replaced by "registerVote" and "retractVote"
- Some newly unreachable/useless code was removed
- "showDetails" state is now handled by AbstractPollResults instead of PollItem

* fix(polls tab): change tab underline color to #525252

* fix(poll create): Enforce at least two options to create a poll

* fix(poll create): change 'remove option' color to #E04757

* fix(poll create): Update Poll create CSS to adapt to design

* fix(poll answer): Adapt CSS to make poll answer closer to mockup

* fix(poll result): Udpdate poll result CSS to match mockups

* fix(poll result): Udpdate poll result CSS to match mockups

* fix(poll create): Display 'remove option' only when there is at least 3 options

* fix(polls button): Add hover, active, focus and disabled state to polls buttons

* Last improvements for web

* Native design fixes

* Fix rebase issue in land/main.json

* Fix french translation after rebase

* Fixmobile behaviour

* Fixed keyboard navigation in web poll creation form

* Fixed Flow error related to "no polls" icon in PollsList

* fix(polls): Enabled polls Prosody module in Debian config files

* doc(polls) Added comments to the Prosody module code

* fix(polls): Switched from using an internal LJM event to ones from the public API

* Capitalize I of setIsPollsTabFocused

* extract the 2 button modes into a const

* remove extra new lines

* Rename CLOSE_POLL_TAB for POLL_TAB_CLOSED for clarity

* Rename answers2 for answersParsed for clarity

* use switch instead of if/else chain

* improve syntax for localId fetching

* Refactor: Use BUTTON_MODE.CONTAINED variable instead of 'contained'

* Disable send poll button if not enough data is provided in the form (#30)

* Feat: Add notification badge on chat and poll tabs (#31)

* Feat: Add notification badge on chat and poll tabs

* Add badge equivalent for native

* Update displayNameForm text to mention polls (#34)

* Disable polls UI with a config in config.js (#33)

* Change remove option text color from red to grey (#32)

Co-authored-by: spineki <[email protected]>
Co-authored-by: Fabien Zucchet <[email protected]>
Co-authored-by: Fabien Zucchet <[email protected]>
Co-authored-by: Fabien Zucchet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Risk - Low Low risk of disruption to codebase or infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants